Adjust invoker playbook to pull docker images when a prefix and tag is specified. #3680

rabbah · 2018-05-18T17:36:48Z

Description

The invoker playbook ignores an image prefix when pre-fetching the blackbox images. This patch splits the blackbox tasks so that ones that do not have a prefix may be skipped (as today), but those that specify a prefix are always pulled, using the specified prefix in the image name.

Fixes #3679.

Related issue and scope

I opened an issue to propose and discuss this change (invoker playbook does not properly pull blackbox images #3679)

My changes affect the following components

Types of changes

Bug fix (generally a non-breaking change which closes an issue).
Enhancement or new feature (adds new functionality).
Breaking change (a bug fix or enhancement which changes existing behavior).

Checklist:

I signed an Apache CLA.
I reviewed the style guides and followed the recommendations (Travis CI will check :).
I added tests to cover my changes.
My changes require further changes to the documentation.
I updated the documentation where necessary.

codecov-io · 2018-05-18T18:25:03Z

Codecov Report

Merging #3680 into master will increase coverage by 0.06%.
The diff coverage is 76.92%.

@@            Coverage Diff             @@
##           master    #3680      +/-   ##
==========================================
+ Coverage   74.92%   74.99%   +0.06%     
==========================================
  Files         132      128       -4     
  Lines        6186     6075     -111     
  Branches      392      388       -4     
==========================================
- Hits         4635     4556      -79     
+ Misses       1551     1519      -32

Impacted Files	Coverage Δ
...er/src/main/scala/whisk/core/invoker/Invoker.scala	`0% <0%> (ø)`	⬆️
...erpool/kubernetes/KubernetesContainerFactory.scala	`0% <0%> (ø)`	⬆️
.../scala/src/main/scala/whisk/core/WhiskConfig.scala	`92% <100%> (-0.25%)`	⬇️
.../containerpool/docker/DockerContainerFactory.scala	`26.66% <100%> (ø)`	⬆️
...rc/main/scala/whisk/core/entity/ExecManifest.scala	`96.29% <100%> (-0.05%)`	⬇️
...scala/whisk/core/mesos/MesosContainerFactory.scala	`61.81% <100%> (ø)`	⬆️
.../scala/src/main/scala/whisk/core/entity/UUID.scala	`81.81% <0%> (-9.1%)`	⬇️
...whisk/connector/kafka/KafkaConsumerConnector.scala	`58.33% <0%> (-3.94%)`	⬇️
...ala/whisk/core/database/CouchDbStoreProvider.scala	`80% <0%> (-1.82%)`	⬇️
...hisk/core/controller/actions/SequenceActions.scala	`30.27% <0%> (-1.16%)`	⬇️
... and 29 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9e5cec1...c600668. Read the comment docs.

dubee

PG4 1715 ⏳

dubee

PG2 3168 ⏳

rabbah · 2018-05-25T03:19:39Z

@csantanapr made the changes we discussed on Slack to disentangle the image pulls from the docker registry.

rabbah · 2018-06-10T22:05:20Z

@dgrove-oss I think you'll need to review this now as its scope as widened a little. I did checked the kube deployment and I think the change is compatible but will appreciate your input.

With the latest changes, runtime images are pulled from the given docker registry for runtimes (default is dockerhub) which is now allowed to be different from the docker.registry; the latter is now strictly used for the system components.

It is possible to skip the pulling of images entirely:

    ansible-playbook -i environments/local invoker.yml -e 'skip_pull_runtimes=true'

or with redo

    redo invoker -e"skip_pull_runtimes=true"

rabbah · 2018-06-10T22:06:41Z

@ddragosd this may also be relevant to docker-compose although I haven't checked that deployment yet.

rabbah · 2018-06-10T22:43:49Z

Some caveats with this change: building new runtime images for local testing means:

locally tagged image used in the runtimes manifest may be used without pushing to docker hub, provided the skip_pull_runtimes is enabled
if skip_pull_runtimes is enabled but the runtimes manifest includes images that are not already pulled, they may fail in the invoker, or be pulled on the first run.

FYI @remore @akrabat @sciabarracom since you will be affected by this change if we accept it.

dgrove-oss · 2018-06-12T20:12:12Z

Looks like this would be straightforward for kube. We'd need to update the copies of runtime.json and tweak values.yaml and the various deployment templates for the pods to reflect the change in environment variables.

csantanapr

LGTM

Remove default runtime prefix and tag; the runtimes.json file should fully specify these. Remove docker.{registry,prefix,tax} entanglement with runtimes as these are used for the system images not the runtimes. Optionally specify a runtimes.registry to pull images from there instead of dockerhub.

…pache#3680) Remove default runtime prefix and tag; the runtimes.json file should fully specify these. Remove docker.{registry,prefix,tax} entanglement with runtimes as these are used for the system images not the runtimes. Optionally specify a runtimes.registry to pull images from there instead of dockerhub.

rabbah assigned csantanapr May 18, 2018

rabbah added the wip label May 18, 2018

dubee reviewed May 18, 2018

View reviewed changes

rabbah force-pushed the invoker-images branch from 665a129 to 56846a0 Compare May 18, 2018 19:03

rabbah added deployment invoker review Review for this PR has been requested and yet needs to be done. and removed wip labels May 18, 2018

dubee reviewed May 22, 2018

View reviewed changes

rabbah added wip review Review for this PR has been requested and yet needs to be done. and removed review Review for this PR has been requested and yet needs to be done. wip labels May 24, 2018

rabbah requested a review from csantanapr May 25, 2018 03:18

rabbah force-pushed the invoker-images branch 3 times, most recently from 6261407 to 584fac4 Compare June 1, 2018 12:04

rabbah changed the title ~~Adjust invoker playbook to pull blackbox docker images when a prefix is specified.~~ Adjust invoker playbook to pull docker images when a prefix and tag is specified. Jun 4, 2018

rabbah mentioned this pull request Jun 6, 2018

Add Ruby2.5 runtime support #3725

Merged

20 tasks

rabbah force-pushed the invoker-images branch from 584fac4 to 0b1adb0 Compare June 6, 2018 14:03

csantanapr force-pushed the invoker-images branch from 0b1adb0 to bb7b911 Compare June 10, 2018 14:07

rabbah force-pushed the invoker-images branch 7 times, most recently from a53a47a to 2b6faf9 Compare June 10, 2018 21:36

rabbah force-pushed the invoker-images branch 6 times, most recently from 3df97d4 to 5fb2565 Compare June 10, 2018 21:59

This was referenced Jun 11, 2018

Remove runtime builds #3745

Merged

Changes to runtimes variables in support of retiring 'whisk/*' runtimes #3412

Closed

Add support for PHP 7.2 runtime #3736

Merged

csantanapr force-pushed the invoker-images branch 2 times, most recently from 3996c6d to fdfde1d Compare June 12, 2018 13:53

csantanapr force-pushed the invoker-images branch 2 times, most recently from 71e2691 to f6e18b1 Compare June 14, 2018 20:55

csantanapr approved these changes Jun 15, 2018

View reviewed changes

csantanapr added pgapproved Pipeline has approved this change. and removed review Review for this PR has been requested and yet needs to be done. labels Jun 15, 2018

rabbah force-pushed the invoker-images branch from f6e18b1 to c600668 Compare June 18, 2018 15:34

dubee approved these changes Jun 18, 2018

View reviewed changes

dubee merged commit 2047c0f into apache:master Jun 18, 2018

This was referenced Jun 18, 2018

Update runtimes for upstream changes. apache/openwhisk-deploy-kube#227

Merged

Update runtimes for upstream changes. apache/openwhisk-devtools#134

Merged

remore added a commit to remore/incubator-openwhisk that referenced this pull request Jun 20, 2018

change data schema as a result of apache#3680

544a996

remore added a commit to remore/incubator-openwhisk that referenced this pull request Jun 21, 2018

remove files which gets unnecessary(deprecated) by apache#3680

39e39ac

rabbah deleted the invoker-images branch June 21, 2018 12:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adjust invoker playbook to pull docker images when a prefix and tag is specified. #3680

Adjust invoker playbook to pull docker images when a prefix and tag is specified. #3680

rabbah commented May 18, 2018

codecov-io commented May 18, 2018 •

edited

Loading

dubee left a comment

dubee left a comment

rabbah commented May 25, 2018

rabbah commented Jun 10, 2018 •

edited

Loading

rabbah commented Jun 10, 2018

rabbah commented Jun 10, 2018 •

edited

Loading

dgrove-oss commented Jun 12, 2018

csantanapr left a comment

Adjust invoker playbook to pull docker images when a prefix and tag is specified. #3680

Adjust invoker playbook to pull docker images when a prefix and tag is specified. #3680

Conversation

rabbah commented May 18, 2018

Description

Related issue and scope

My changes affect the following components

Types of changes

Checklist:

codecov-io commented May 18, 2018 • edited Loading

Codecov Report

dubee left a comment

Choose a reason for hiding this comment

dubee left a comment

Choose a reason for hiding this comment

rabbah commented May 25, 2018

rabbah commented Jun 10, 2018 • edited Loading

rabbah commented Jun 10, 2018

rabbah commented Jun 10, 2018 • edited Loading

dgrove-oss commented Jun 12, 2018

csantanapr left a comment

Choose a reason for hiding this comment

codecov-io commented May 18, 2018 •

edited

Loading

rabbah commented Jun 10, 2018 •

edited

Loading

rabbah commented Jun 10, 2018 •

edited

Loading